Skip to content

gh-118909: Fix crash freeing tp_doc allocated with PyObject_Malloc() - #156990

Closed
NAVEENKUMARKR777 wants to merge 1 commit into
python:mainfrom
NAVEENKUMARKR777:gh-118909-tp-doc-free-mismatch
Closed

gh-118909: Fix crash freeing tp_doc allocated with PyObject_Malloc()#156990
NAVEENKUMARKR777 wants to merge 1 commit into
python:mainfrom
NAVEENKUMARKR777:gh-118909-tp-doc-free-mismatch

Conversation

@NAVEENKUMARKR777

@NAVEENKUMARKR777 NAVEENKUMARKR777 commented Sep 5, 2026

Copy link
Copy Markdown

Summary

Fixes gh-118909. type_dealloc() always frees a heap type's tp_doc with PyMem_Free(), but some C extensions (older versions of pybind11, nanobind, datatable) allocate it directly with PyObject_Malloc() instead, relying on CPython to free it. In a release build both domains share the same underlying allocator, so this goes unnoticed, but a build with debug allocator hooks enabled (Py_DEBUG, or PYTHONMALLOC=debug) tags each domain's blocks and aborts the process when a block is freed with the wrong domain (Fatal Python error: _PyMem_DebugRawFree: bad ID: Allocated using API 'o', verified using API 'm').

This implements the approach discussed by @colesbury and @erlend-aasland on the issue ("option 3"): detect which allocator was actually used, from the one-byte domain tag the debug allocator writes just before the returned pointer, and free with the matching function as a documented, backwards-compatibility fallback (not a stable guarantee).

Changes

  • Objects/typeobject.c: add type_free_tp_doc(), used in type_dealloc().
  • Modules/_testcapi/heaptype.c + Lib/test/test_capi/test_mem.py: regression test that builds a heap type with a PyObject_Malloc()-allocated tp_doc and deallocates it under the debug allocator.
  • Doc/c-api/typeobj.rst: document the tp_doc allocation contract and the fallback.
  • Misc/NEWS.d: changelog entry.

Test plan

  • New test (test_pyobject_malloc_tp_doc) reproduces the exact reported crash when run against the pre-fix code, and passes with the fix, under malloc_debug, pymalloc_debug, and mimalloc_debug.
  • test_capi, test_types, test_descr (1,820 tests) pass with no regressions.

🤖 Generated with Claude Code

@NAVEENKUMARKR777 @claude
pythongh-118909: Fix crash freeing tp_doc allocated with PyObject_Mal…
c9c8a40 
…loc()

type_dealloc() always freed a heap type's tp_doc with PyMem_Free(),
but some C extensions (e.g. older pybind11 and nanobind versions)
allocate it directly with PyObject_Malloc() instead, relying on
CPython to free it. The two allocator domains share the same
underlying allocator in a release build, so this went unnoticed, but
a build with debug allocator hooks enabled (Py_DEBUG, or
PYTHONMALLOC=debug) tags each domain's blocks and aborts when a block
is freed with the mismatched domain.

Detect which allocator was actually used, from the tag debug builds
write before the returned pointer, and free with the matching
function as a backwards-compatibility fallback.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@python-cla-bot

python-cla-bot Bot commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app bedevere-app Bot mentioned this pull request Sep 5, 2026
Open
@read-the-docs-community

read-the-docs-community Bot commented Sep 5, 2026

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #34408479 | 📁 Comparing c9c8a40 against main (e620377)

  🔍 Preview build  

2 files changed
± c-api/typeobj.html
± whatsnew/changelog.html

ZeroIntensity
ZeroIntensity requested changes Sep 5, 2026
View reviewed changes

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix only applies to debug builds, which isn't very useful. The cases that we care about (i.e., in third-party extensions) use release builds.

@bedevere-app

bedevere-app Bot commented Sep 5, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@NAVEENKUMARKR777

NAVEENKUMARKR777 commented Sep 5, 2026

Copy link
Copy Markdown
Author

@ZeroIntensity Thanks for taking a look. I want to make sure I understand the concern correctly before changing anything, since I think release builds are actually fine here today — happy to be corrected.

I went through every allocator backend CPython ships to check whether PyMem_Free()/PyObject_Free() actually diverge outside of debug mode:

  • pymalloc (the default non-free-threaded build): PYMEM_DOMAIN_MEM and PYMEM_DOMAIN_OBJ are assigned the literal same allocator struct (PYMALLOC_ALLOC) in set_up_allocators_unlocked().
  • mimalloc (the default free-threaded build): PYMEM_DOMAIN_MEM's free function is _PyMem_MiFree, PYMEM_DOMAIN_OBJ's is _PyObject_MiFree — both are just mi_free(ptr), byte-for-byte identical (Objects/obmalloc.c:296 and :379). I also checked whether a third-party PyObject_Malloc() call could land a plain (non-PyObject) buffer in the special GC-scanned mimalloc heap and corrupt the free-threaded GC's heap-walking — it can't, since current_object_heap is only pointed at the GC heap for the literal duration of _PyObject_MallocWithType()'s own call and reset immediately after (Include/internal/pycore_object_alloc.h:38-51); a direct, un-wrapped PyObject_Malloc() call always lands in the ordinary _Py_MIMALLOC_HEAP_OBJECT, which gc_visit_heaps() never scans.
  • plain malloc (--without-pymalloc or PYTHONMALLOC=malloc): both domains are plain malloc/free.

So in every release-mode configuration I can find, tp_doc freed with the "wrong" domain's free function is safe today — the crash is specific to the debug allocator wrapper, which matches how the bug was originally reported ("crashes when using pybind11 in debug builds of Python 3.13").

Given that, I want to make sure I'm addressing the right concern:

  1. Is the worry that PYTHONMALLOC=debug/Py_DEBUG builds aren't representative of what third-party extension users actually hit? (For what it's worth, PYTHONMALLOC=debug runs against a normal release-compiled Python, so this isn't limited to --with-pydebug builds — it's a common way people actually bisect crashes like this one.)
  2. Or is it more that today's release-mode safety is coincidental (an artifact of the current allocator implementations aliasing each other), not a real guarantee, and you'd want something that holds up even if that changes? If so, I think that's a harder problem worth naming explicitly: detecting the allocation domain in a non-debug build isn't possible without extra metadata that isn't there today, so a "durable" fix would need a different shape entirely (e.g. reviving colesbury's option 4 from the original discussion — a dedicated PyMem_DocMalloc/PyMem_DocFree-style API — or the critical-section-callback sketch he mentioned).

Let me know which direction you'd like, or if there's a concrete release-build crash scenario I'm missing — happy to dig further.

@picnixz

picnixz commented Sep 5, 2026

Copy link
Copy Markdown
Member

@NAVEENKUMARKR777 Avoid poor LLM replies. We don't want to converse with an agent.

@ZeroIntensity

ZeroIntensity commented Sep 5, 2026

Copy link
Copy Markdown
Member

Yes, please use your own words!

  1. People can override the allocators with whatever they want. Fundamentally, we shouldn't rely on the idea that PyObject_Free and PyMem_Free point to the same allocator.
  2. We should address the strdup/malloc case as well, as that continues to crash.

I think this needs more discussion before diving into fixes.

@picnixz picnixz closed this Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZeroIntensity ZeroIntensity ZeroIntensity requested changes
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

tp_doc switch from PyObject_Malloc to PyMem_Malloc is not backwards compatible

3 participants

@NAVEENKUMARKR777 @picnixz @ZeroIntensity